Review requested:

• @nodejs/nodejs-website
• @nodejs/web-infra

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.73%. Comparing base (3819c7f) to head (5956e06).
⚠️ Report is 8 commits behind head on main.

@@            Coverage Diff             @@
##             main   #57343      +/-   ##
==========================================
- Coverage   89.74%   89.73%   -0.01%     
==========================================
  Files         675      675              
  Lines      204642   204642              
  Branches    39322    39327       +5     
==========================================
- Hits       183657   183642      -15     
- Misses      13257    13287      +30     
+ Partials     7728     7713      -15     

No it does apply, in the test file we have Object.values(json).at(-1)?.[0].introduced_in – because the JSON output has the structure it has, the key to access the file content varies depending on what's being documented (most of the times, it's modules, sometimes it's miscs, and once it's globals), the only invariant being that it's always the last key of the top-level object. If the keys get re-ordered and it's no longer the last key, it's a potential breaking change.
Why risking introducing a breaking change when it's so easy to re-order the keys when generating the JSON? I don't get it.

And FWIW I definitely agree that the current JSON output is in a less than ideal, and a reform would be welcome – but in a follow up semver-major PR.

I wanted to clarify a few points regarding the JSON structure and why the ordering has changed.

First, it's important to note that the top-level keys (which dictate the content order from the source Markdown) remain intact. We aren't introducing any breaking changes to the overall sequence of the document content.

Regarding the inner keys of each entry:

1. The ordering in the legacy tooling isn't actually semantic or "factually correct" based on the docs; it's simply an artifact of the order in which the legacy script tests for features.
2. In the new tooling, we handle these features in a different order simply because that's how the new logic was structured.

Since JSON objects are inherently unordered, this change shouldn't break any standard parsing. Unless a downstream consumer is accessing keys by index (e.g., Object.keys(json)[0]) rather than by name (e.g., json.type), the impact is purely visual.

While we can refactor the new tooling to mirror the legacy order if it’s considered a blocker, it feels like an unnecessary use of resources for a change that doesn't provide functional value. The current test failures seem to be a result of the tests expecting a specific string-matching order rather than validating the schema itself.

@aduh95, could you provide a real-world example of where this specific internal key order would break downstream usage? It would help to understand if there's a dependency I'm overlooking.

To clarify for everyone else, the concern is that keys like "type" appear higher up in the new output compared to the legacy output:

Note: The highlighted keys are just examples of the shift in placement. Even in the legacy tooling, this order wasn't always 100% consistent across different files.

could you provide a real-world example of where this specific internal key order would break downstream usage?

Me trying to review this PR. With keys in a different order, it's very hard to review. With nodejs/doc-kit#595, at least I'm able to diff which allows me to see that e.g. node:process's 'message' event params are no longer parsed correctly:

The HTML version is also suffering from the lack of parsing:

I haven't investigated, that's likely due to the conflict on tools/doc/type-parser.mjs rather than an actual bug in doc-kit, hopefully though it does demonstrate how important it is for reviewing.

The sub-keys being in a different order does produce a lot of diff in the output, it's annoying but I guess it's workable. BTW there are indeed lots of bug fixes, so good work!

Me trying to review this PR. With keys in a different order, it's very hard to review. With nodejs/doc-kit#595, at least I'm able to diff which allows me to see that e.g. node:process's 'message' event params are no longer parsed correctly:

That's an issue with the markdown. See #61756

Me trying to review this PR. With keys in a different order, it's very hard to review. With nodejs/doc-kit#595, at least I'm able to diff which allows me to see that e.g. node:process's 'message' event params are no longer parsed correctly:

Gotcha, for manual comparison of the JSON this makes sense. I guess we can land nodejs/doc-kit#595

Me trying to review this PR. With keys in a different order, it's very hard to review. With nodejs/doc-kit#595, at least I'm able to diff which allows me to see that e.g. node:process's 'message' event params are no longer parsed correctly:

That's an issue with the markdown. See #61756

Do you mean to say the "node:process's 'message' event params are no longer parsed correctly:" is an issue on the markdown or?

Do you mean to say the "node:process's 'message' event params are no longer parsed correctly:" is an issue on the markdown or?

Yes. The new tooling is much stricter on what it considers valid vs invalid. The markdown for that particular event would be considered invalid by the new tooling.

Once this PR lands, we can land the new linter, which will identify invalid markdown before they land.

@aduh95 hopefully all your concerns are resolved?

Do you mean to say the "node:process's 'message' event params are no longer parsed correctly:" is an issue on the markdown or?

Yes. The new tooling is much stricter on what it considers valid vs invalid. The markdown for that particular event would be considered invalid by the new tooling.

Once this PR lands, we can land the new linter, which will identify invalid markdown before they land.

I'd argue we should fix that specific Markdown file before we land this PR, otherwise that section is broken.

See #61756

A few bugs I've found while browsing the JSON diff:

When a function has multiple signatures, on this PR it's no longer taken into account, the JSON no longer contains info about e.g. return type or history

REPLACEME is for some reason considered older instead of newer (FWIW we have a linter rule that enforces the order in the markdown is correct, so the markdown order should be used instead)

The tool identify as params things that clearly could not be valid JS identifiers

Also it looks like #57516 was never ported to doc-kit (i.e. having stability labels sticking to the top of the page), any reason for not doing it?

I guess the first item is potentially breaking, do we know how easy/hard is it to fix? I don't think it's a blocker for landing, it might be one for backporting though. The other ones are nice-to-have, and should be easy to fix anyway.

When a function has multiple signatures, on this PR it's no longer taken into account, the JSON no longer contains info about e.g. return type or history

It still does, however, it correctly places the return type, history, etc with the signature it is associated with. (i.e. you'll find all the meta items within the signature array.

is for some reason considered older instead of newer (FWIW we have a linter rule that enforces the order in the markdown is correct, so the markdown order should be used instead)

Noted. We can just remove the sortChanges function. However, REPLACEME changes are never included in the final release, so it shouldn't really matter

The tool identify as params things that clearly could not be valid JS identifiers

There's a give/take here. The more exclusive the parsing is, the higher the chance of a false-negative. The less exclusive, the higher the chance of a false positive. That being said, I don't think it'll break anything if we exclude [0-9'"] from starting the RegEx in the CAMEL_CASE constant.

It still does, however, it correctly places the return type, history, etc with the signature it is associated with. (i.e. you'll find all the meta items within the signature array.

There are two cases:

• each signature has its own doc (e.g. process.emitWarning):
  • ✅ current tooling attaches different metadata (history, return type, params) to each.
  • ✅ doc-kit attaches different metadata (history, return type, params) to each.
• both signatures share the same doc (e.g. stream.pipeline):
  • ✅ current tooling interprets it as a fall-through and attach same metadata to both.
  • ❌ doc-kit attaches no metadata to the first signature.

That's a regression, not a major one, but still one worth fixing before we can safely backport.

REPLACEME changes are never included in the final release, so it shouldn't really matter

It definitely matters for the DX though

It definitely matters for the DX though

It is called time travel, Antoine, never heard of? Future versions actually came before! 😛